feat(spec,metadata-protocol): put the runtime authoring gate's advisory findings on the save response (#4717) - #7435
Conversation
…ry findings on the save response (#4717) #4463 put the shared author-time rule registry on the runtime write path — the fourth door, and for a Studio tenant or an MCP/AI author the ONLY one. It gated on `error` findings only; the advisory half was produced, walked into a `console.warn` deduped once per process, and dropped. PR #4715 named that when it shipped: running a rule and discarding its conclusion is a smaller version of the hole the gate exists to close. The case is reachable, not theoretical. A flow whose only defect is a `delete_record` node declaring `multi: true` with no `filter` yields `errors = 0 / advisories = 1`: the write succeeds, the row persists, and the author never learns their nightly sweep deletes every row on every run. - `RuntimeAuthoringIssueSchema` declares the finding shape ONCE, in spec — the same `rule`/`path`/`where`/`message`/`hint`/`severity` the 422 envelope carries (#4463 D3). `metadata-protocol` re-exports it as its `RuntimeAuthoringIssue` instead of keeping a parallel hand-written interface. - `SaveMetaItemResponseSchema.advisories` — optional, array of that element. - `evaluateRuntimeAuthoringGate` returns a `RuntimeAuthoringVerdict` (`{ error, advisories }`) instead of `Error | null`. An ADDED return channel: the success path previously returned `null` and had nowhere to put a verdict. - `saveMetaItem` attaches the advisories, emitting the key ONLY when non-empty — never `[]` — so a clean save's response bytes are unchanged. `rulesRun` stays off: the gate appends a gate-local rule id for `flow`, so not every id it would list resolves in the lint registry. Save door only; the publish door's asymmetry is stated in the changeset. Fixes #4717 Claude-Session: https://claude.ai/code/session_01W6bLax4KMrSfnE1ydFU8Dw Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 2 package(s): 106 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
ACCEPT — PM review (step 7)File surface14 files, which reconciles with the report's "6 changed source files" (that figure is its ESLint scope — 3 source + 3 test):
All seven generated entries are CI, per job26 check runs, all The four ruled constraints, each verifiable in the diff
One thing better than I asked for: rather than adding the field alongside the existing hand-written The cross-seat
|
Fixes #4717
The metadata half only, per the maintainer's 2026-08-07 migration ruling: advisory findings reaching the
saveMetaItemresponse envelope. The lint "surfaces" generalisation (spec-tooling), the Studio rendering (repo:objectui) and the docs "the gate" fourth column are not in this PR.packages/spec, and #6017 owns that surfaceTwo files under
packages/spec/src/are changed here by thedomain:metadataseat. Calling it out at the top rather than leaving it to the diff, because the maintainer's 2026-08-10T07:33Z ruling namesdomain:spec(#6017) as the actor for this declaration, and only the typist is different here — not the substance.packages/specfiles as a correct application. Those landed at 09:03Z with no objection raised.The spec files, and nothing else under
packages/spec/src/:packages/spec/src/api/protocol.zod.tsRuntimeAuthoringIssueSchema(new export) +advisoriesonSaveMetaItemResponseSchema(optional) + theRuntimeAuthoringIssuetype aliaspackages/spec/src/type-alias-convention.pin.test.tsIso837), 825 → 826What was broken
#4463 put the shared author-time rule registry (
AUTHORING_RULES— the same tableos validate/os build/os lintrun) on the runtime write path. It gated onerrorfindings only. The advisory half was produced atpackages/lint/src/runtime-gate.ts, walked into aconsole.warndeduped once per process pertype|name|rule|pathatpackages/metadata-protocol/src/runtime-authoring-gate.ts, and then went out of scope. PR #4715 named the shape when it shipped: running a rule and discarding its conclusion is a smaller version of the hole the gate exists to close.The failing case is reachable today, not theoretical. A
nightly_purgeflow whose only defect is adelete_recordnode declaringmulti: truewith nofilteryieldserrors = 0 / advisories = 1. The gate allows it, the write succeeds, the row persists, the flow registers — and the author never learns that their nightly sweep deletes every row of the object on every run. A Studio tenant or an MCP/AI author has noos lintand no CLI config file forsys_metadataoverlay rows, so for them this gate is not the weakest of four doors. It is the only one.What this changes
This is an ADDED return channel, not a threaded value.
evaluateRuntimeAuthoringGatewas typed=> Error | nulland returnednullon success — the success path had nowhere to put a verdict at all. It now returns aRuntimeAuthoringVerdictrecord{ error, advisories }. A record rather than a widened union, deliberately: both halves are always answered, and a caller cannot read one while forgetting the other exists.The element shape is declared once.
RuntimeAuthoringIssueSchemain spec is now the single declaration of the six keys (rule/path/where/message/hint/severity), andmetadata-protocolre-exports it as itsRuntimeAuthoringIssueinstead of keeping a hand-written interface for the same shape next to a Zod schema. The 422'sissues[]and the 2xx'sadvisories[]cannot drift into two dialects. Per the ruling: reuse it, do not mint a parallel shape.Emitted only when non-empty. The key is omitted — never
[]— so a clean save's response bytes are byte-for-byte what they were before, and a caller that ignores the field behaves exactly as today.rulesRunstays off. The gate appends the gate-localPLATFORM_SCHEDULE_CREATE_RECORD_ORG_MISSINGwhentype === 'flow', so not every id it would list resolves in the lint registry; shipping the array would require the spec.describe()to state that the ids are gate ids. A field can be added later, not removed.Save door only, and the asymmetry is stated in the changeset. The gate runs on both write doors on purpose (
saveMetaItemand the draft→active promotion, so?mode=draft+ publish is not a bypass — #4463 D1). The publish door does not carry this field; it rides behind #7294. The promotion door's call site simply ignores the new return value, which is why adding the channel could not change what that door does.File surface, file by file
Contract (
packages/spec) — the cross-seat half declared abovepackages/spec/src/api/protocol.zod.ts—RuntimeAuthoringIssueSchema,advisoriesonSaveMetaItemResponseSchema,RuntimeAuthoringIssuealias.packages/spec/src/type-alias-convention.pin.test.ts—Iso837+ the three count statements (pin list header, case title,toHaveLength).Producer (
packages/metadata-protocol)packages/metadata-protocol/src/runtime-authoring-gate.ts— the interface becomes a re-export of the spec type; newRuntimeAuthoringVerdict; all four return points converted. The deduped operator log is kept, it is simply no longer the only channel.packages/metadata-protocol/src/protocol.ts—assertRuntimeAuthoringRulesreturnsRuntimeAuthoringIssue[](throws unchanged);saveMetaItemcaptures it into a local and spreads it conditionally onto the success return.Tests
packages/objectql/src/save-meta-response-conformance.test.ts— 5 new cases through the REAL protocol / REAL engine / REALrunRuntimeAuthoringRules.packages/spec/src/api/protocol.test.ts— 7 new cases pinning the declaration itself.Generated artifacts — regenerated, never hand-edited
packages/spec/api-surface/api.json,packages/spec/export-origins/api.json,packages/spec/authorable-surface/api.json,packages/spec/json-schema.manifest/api.jsondocs/audits/2026-07-unknown-key-strictness-ledger.counts.md(api/397 → 398; the row'swireverdict already covers a response envelope, so the prose needed no change)content/docs/references/api/protocol.mdx,content/docs/references/index.mdxChangeset
.changeset/runtime-gate-advisories-on-save-response.md—minoron both packages, and it states the save/publish asymmetry so it is not read as an oversight.content/docs/releases/is untouched.Reverse verification
Every direction was declared before the probe ran, against a fixed base —
origin/main@f16e54e1d7851770748c927c85db53aa73bbd722, never a moving ref. Nogit stashat any point: the fix was taken out and put back with file copies inside my own worktree.1. Predicted red, came out red
advisoriesline fromSaveMetaItemResponseSchema(producer untouched)strippedKeysreports the keyAssertionError: expected [ 'advisories' ] to deeply equal [],Tests 1 failed/9 passedAssertionError: Target cannot be null or undefined.,Tests 1 failed/9 passedadvisories: runtimeAdvisoriesunconditionally — the well-meaning[]refactorTests 3 failed/7 passed, all three reds mineadvisories→advisoryFindings)Tests 1 failed/9 passedhintfrom the shared element schemaAssertionError: expected [ { …(5) } ] to deeply equal [ { …(6) } ]R4 is the load-bearing one. It is the exact violation the ruling's point 2 forbids, and it demonstrates that the pre-existing conformance gate is structurally blind to it: all five #5745 cases stayed green while
[]was being emitted on every save. Only the new absence pins caught it.R5, stated precisely: the direction was right and the cause was the rename, but the message was not the one I predicted. I predicted
strippedKeyswould report['advisoryFindings']; in fact thetoHaveLength(1)assertion fires first and the run stops there. Reporting the assertion that actually failed rather than the one I expected.2. Missed predictions
R1's first run was INVALID and I nearly recorded it as a pass. I removed the spec field, rebuilt spec with output redirected to
/dev/null, and gotTests 10 passed. The spec build had failed, so the test read a staledist— the green was measuring the fix, not the counterfactual. Cause: never send stderr to/dev/nullwhen the output is evidence. Re-run visibly, the cause turned out to be a third diagnostic I had not predicted at all:build-schemas.ts's authorable-surface ratchet refuses the removal at build time, before any test runs —So once this lands, deleting the field is red at two independent layers, one of which never reaches a test runner. R1 above is the honest counterfactual: field and baseline line removed together, i.e. "the declaration was never added".
R6′ missed its type-level half. I predicted that narrowing the shared element shape (dropping
hint) would turnmetadata-protocolred, proving the re-export is genuinely load-bearing at the type level rather than cosmetic. It did not: the package's DTS build succeeded. Cause, measured rather than guessed —packages/metadata-protocolhas notypecheckscript (it sits in thecheck:type-check-coverageDEBT ledger) and tsup never type-checks (AGENTS.md, #4311). There is no tsc program over that file for a narrowed type to redden. The runtime half of the same probe does go red (table above), so the schema really is the one used at parse; the type-level claim is simply not checked anywhere today, and I would rather say so than let the re-export read as more guarded than it is.3. Assertions green in BOTH directions — guards, not evidence
The variant the previous dev flagged as explicitly unmeasured, now measured. Does a conditionally added key leave the three pre-existing conformance cases green? Yes — and in both directions, which is the point: under R1 (no spec declaration at all) they were green; with the full fix they are green. They save clean
viewbodies, the key is absent, andstrippedKeysis empty either way.That is labelled as a guard in the test file itself, not only here — the case is literally named
GUARD (green either way): a clean view save is untouched by the new field, and its docblock states that the existing gate cannot go red to report a missing or misspelled declaration. The reason it is written down rather than left out: a future reader seeing five green conformance cases would otherwise reasonably conclude the new field is covered by them. It is not. It is covered by the two directional cases added next to it.4. Predictions left unmeasured
=> Error | nullrestored on the gate signature) — not run. It would be a trivial compile error at the four converted return sites and carries no information the R2 probe does not already carry. Declared, not measured; not counted as passed.rest-server.tshands this exact object tores.json()verbatim, so the protocol return IS the wire body — I did not re-derive that with a live server, and I am not claiming an HTTP-level measurement I did not take. The "still a 200" assertion in the new case is the protocol-level one:saveMetaItemthrows on the gated half, so reaching the success assertions at all is the statement that this write was not refused.POST /meta/:type/:name/publishis a served REST route with NO spec declaration — the #5745 "declared = returned" discipline covers only the save door #7294 owns it.Tests
All run in the worktree, real output:
packages/lintis included becauseauthoring-rule-wiring.test.tsscans the gate file's imports, and this PR adds one.Typecheck —
pnpm --filter @objectstack/spec --filter @objectstack/objectql typecheckclean (metadata-protocoldeclares notypecheckscript; see §2 above). ESLint clean on all six changed source files. Gates run locally, all green:check:api-surface,check:export-origins,check:authorable-surface,check:strictness-ledger,check:docs,check:spec-parsed-alias(826 pinned isomorphic),check:route-envelope,check:error-code-casing,check:engine-double-contract,check:meta-type-normalized,check:adr-anchors,check:doc-authoring,check:type-check-coverage,check:changeset-gate-self-tests,check:nul-bytes.Notes for review
codeANDstatus({ code: 'INVALID_METADATA', status: 422 }), never a baretoThrow(). That body already threw before this change, so a throw-only assertion would have been permanently green and could not have shown that the refusal did not move when the success path grew a channel.OS_ALLOW_UNLINTED_METADATA_WRITES=1converts a refusal into a log;advisoriesmeans "did not block this write", and a finding that only failed to block because an operator set a migration flag is not that. Widening it would putseverity: 'error'entries on a 2xx. Stated in the code at the return site.allowOrgOverride: falseforflow, so an org-scoped write is refused403 NOT_OVERRIDABLElong before the gate runs.runAs: 'system'on the advisory fixture is load-bearing too — without itflow-runas-unscopedfires aterrorand the case would pass for the wrong reason (a refusal wearing an advisory's clothes). Both are commented in the test.erroronly, so it adds nothing to the advisory channel.🤖 Generated with Claude Code
Generated by Claude Code